-
-
Notifications
You must be signed in to change notification settings - Fork 784
[17.0][OU-FIX] account: assign xmlid to journals #5163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
402824b to
2cc3508
Compare
pedrobaeza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may be overcomplicating the code. I think you just need to look for the first journal of each type and assign the XML-ID. Matching per name doesn't worth, as you may rename the journals, install them in another language, or a lot of reasons.
|
And please put the other fix in a separate PR, or at least commit. |
2cc3508 to
4b82052
Compare
4b82052 to
2241eac
Compare
|
Hi @pedrobaeza thanks for your comments, I moved fix on assign chart_template to #5166.
|
|
@remi-filament I still think it should be simplified, as all that code Odoo core is if not XML-ID is found, but here we want to just assign one for not triggering that code, so it's enough assigning one journal, not matter which. |
8284f12 to
a918cf1
Compare
|
Fine @pedrobaeza, I was afraid that maybe somewhere in the code Odoo is trying to get account_journal by XML-id and would fail if we set the incorrect one, but maybe it does not happen. Let me know if this is OK for you ? |
a918cf1 to
0162875
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope this fix is fine and we don't need to add more conditions (like the code) in the filtered later.
This PR fixes #5108 for duplicate journals
When testing this PR I also realized there was a problem assigning chart_template from chart_template_id since company_id was retrieved instead of chart_template_id. I fixed that as well.
I had to copy a few lines from Odoo account/models/chart_template.py code, I added information about that lines in comments, let me know if you think this is not acceptable ?